fix(Migrator): don't panic in AlterColumn/DropColumn when value is a table-name string - #234

…table-name string

The base GORM migrator accepts a table-name string as the value for
migrator methods (RunWithValue sets stmt.Table and leaves stmt.Schema
nil), and the other dialects support this. The sqlite driver
dereferenced stmt.Schema unconditionally in AlterColumn and DropColumn,
so both panicked with a nil pointer dereference:

    db.Migrator().DropColumn("my_table", "my_column")

DropColumn works fine without a schema (the column name is used as-is),
so it now just skips the field lookup. AlterColumn cannot build the new
column type without the model schema and returns an error instead of
panicking.

The test closes the pool on cleanup so the shared in-memory DB is torn
down and repeated runs (go test -count=N) start from a clean state.

Originally submitted as go-gorm#234.

)

* fix(Migrator): don't panic in AlterColumn/DropColumn when value is a table-name string

The base GORM migrator accepts a table-name string as the value for
migrator methods (RunWithValue sets stmt.Table and leaves stmt.Schema
nil), and the other dialects support this. The sqlite driver
dereferenced stmt.Schema unconditionally in AlterColumn and DropColumn,
so both panicked with a nil pointer dereference:

    db.Migrator().DropColumn("my_table", "my_column")

DropColumn works fine without a schema (the column name is used as-is),
so it now just skips the field lookup. AlterColumn cannot build the new
column type without the model schema and returns an error instead of
panicking.

The test closes the pool on cleanup so the shared in-memory DB is torn
down and repeated runs (go test -count=N) start from a clean state.

Originally submitted as #234.

* fix(ddlmod): recognize the ? placeholder in constraint clauses

CreateConstraint appends the constraint to the DDL field list in the
form produced by constraint.Build(): `CONSTRAINT ? FOREIGN KEY ...`.
constraintRegexp required a backquoted constraint name, so getColumns
did not filter the clause out and extracted the literal `CONSTRAINT` as
a column name; the rebuild's data copy then failed with

    table x__temp has no column named CONSTRAINT

which breaks AutoMigrate whenever an existing table is missing a
foreign key declared in the model. The regexp now accepts a quoted or
unquoted name as well as the ? placeholder, matching the more tolerant
compileConstraintRegexp used elsewhere.

Originally submitted as #241.

* fix: quote string literals with single quotes in Explain

Explain wrapped string values in double quotes, which are identifier
quoting in SQLite — SQL copied from the logs misparses the value as a
column reference. Use standard single-quoted string literals instead.

GORM also embeds string default values into CREATE TABLE through
Explain, so the DDL parser now strips single quotes from parsed
default values as well (double quotes are still stripped for tables
created by older driver versions); without that, migrations would stop
being idempotent. Only one matching outer pair is stripped, so inner
quotes of values like '"x"' survive. Covered by a default-value
round-trip test.

Fixes #197
Originally submitted as #242.

* fix(Migrator): don't let rows.Close() overwrite ColumnTypes errors

The deferred `err = rows.Close()` unconditionally replaced the named
return value, so any error from rows.ColumnTypes() or the merge loop
was swallowed by the (almost always nil) Close result. Keep the first
error instead.

Originally submitted as #243.

* fix: don't reduce a composite primary key to its autoIncrement column

DataTypeOf emitted "integer PRIMARY KEY AUTOINCREMENT" for any
auto-increment int field. With a composite primary key GORM then skips
the table-level PRIMARY KEY clause (the field type already contains
PRIMARY KEY), so the other key columns silently lost their primary-key
status. AUTOINCREMENT only applies to a single-column INTEGER PRIMARY
KEY anyway, so it is dropped when the field is part of a multi-column
key and the table-level PRIMARY KEY (a, b) is emitted instead.

The single-key behavior — including autoIncrement without an explicit
primaryKey tag — is unchanged.

Originally submitted as #244.

* fix(ddlmod): parse parameterized types with precision like decimal(10,2)

The type group of columnRegexp had no comma, so `decimal(10,2)` was
truncated to `decimal(10` in ColumnType(), and the scale was lost.
The type group now takes the whole parenthesized parameter list, and
the size parsing distinguishes a single length (varchar(10) -> Length)
from precision and scale (decimal(10,2) -> DecimalSize/Scale), spaces
after the comma included.

Originally submitted as #245.

* fix(Migrator): harden DDL matching and error reporting

Six related fixes to how the migrator matches DDL and reports failures:

- GetTables: exclude sqlite_* internal tables; sqlite_sequence appeared
  in the list as soon as any table used AUTOINCREMENT.
- HasConstraint: match the exact constraint name via the parsed DDL and
  compileConstraintRegexp instead of LIKE substrings, which matched
  prefixes and names embedded in other text. The regexp accepts
  backquotes, double quotes, single quotes, brackets or no quoting, so
  every form the LIKE patterns handled keeps working.
- DropColumn: report an error when the column is not present in the
  DDL; removeColumn returned false silently and the table was rebuilt
  unchanged.
- removeColumn: match unquoted column names; the regexp required a
  quote or space before the name, so columns of hand-written DDL like
  "CREATE TABLE t (id integer, name text)" were never matched.
- getRawDDL: report "table not found" instead of passing an empty
  string to parseDDL, which surfaced as a confusing "invalid DDL".
- uniqueRegexp: recognize lowercase and no-space table-level
  unique(...) constraints; every other DDL regexp is already
  case-insensitive.

* fix(ddlmod): recognize bracket-quoted constraint names

Address review feedback: brackets were the one quoting form neither
constraintRegexp nor uniqueRegexp accepted, so
`CONSTRAINT [chk_a] CHECK (a > 0)` fell through to getColumns and was
picked up as a bogus `CONSTRAINT` column — the same rebuild failure
("table x__temp has no column named CONSTRAINT") the ? placeholder
caused. A bracket-named table-level UNIQUE also lost the unique flag on
its columns for the same reason.

Both regexps now accept [name] alongside the quoting forms they already
handled, matching compileConstraintRegexp. TestConstraintNameQuoting
additionally asserts that no constraint clause leaks into getColumns,
and TestUniqueConstraintNameQuoting covers UNIQUE across all forms.

* fix(ddlmod): accept hyphens and brackets in identifier matching

Address the remaining review comments.

- constraintRegexp only allowed [\w\d_] in a constraint name, so a
  quoted name containing a hyphen was not recognized as a constraint
  clause and getColumns picked up a bogus `CONSTRAINT` column — the same
  rebuild failure as the bracket and ? placeholder cases. uniqueRegexp
  already allowed hyphens.
- removeColumn and the column-name fallback in getColumns did not accept
  bracket-quoted identifiers. Since DropColumn now reports an error when
  removeColumn finds nothing, a bracket-quoted column went from a silent
  no-op to a hard "not found in the DDL" failure; getColumns dropped it
  from the rebuild's copy list as well.

HasConstraint no longer returns errors from its RunWithValue callback.
The result is a plain bool and the callback's error is discarded by the
caller, so a missing table or unreadable DDL now simply answers false.
This is a clarity fix rather than a behavior fix: gorm's RunWithValue
only returns the error, it does not record it on the session, so the
discarded error was never observable — verified by
TestHasConstraintMissingTable, which passes with and without the change
and guards that the session stays usable.

Note that parseDDL still cannot read a bracket-quoted *table* name
(tableRegexp), which predates this PR and is left alone here.

* fix(ddlmod): match non-ASCII constraint names

constraintRegexp allowed only [\w\d_-] in a constraint name, which is
ASCII, so a quoted non-ASCII name was not recognized as a constraint
clause and getColumns picked up a bogus `CONSTRAINT` column — the same
rebuild failure as the hyphen, bracket and ? placeholder cases.

* fix(ddlmod): read bracket-quoted and non-ASCII identifiers

SQLite identifiers may be quoted with brackets and are not restricted to
ASCII, but the DDL regexps only covered the quote characters and \w. The
character classes now match what SQLite accepts:

- columnRegexp: bracket-quoted and non-ASCII column names were not
  parsed into ddl.columns at all, so ColumnTypes did not report them and
  a table rebuild dropped them from the copy list — losing their data.
  The getColumns fallback gained the same treatment.
- tableRegexp: a bracket-quoted table name failed the whole parse with
  "invalid DDL", so HasConstraint, DropColumn and AlterColumn all failed
  on such a table. renameTable needs the same treatment: it rewrites the
  name inside the parsed head, and with the brackets left in place the
  rebuilt head became CREATE TABLE [ `t__temp` ], which SQLite reads as
  a table whose name contains backquotes.
- indexRegexp: ColumnTypes feeds parseDDL the table DDL together with
  every index DDL, so a bracket-quoted or non-ASCII index name failed
  the whole call with "invalid DDL".
- uniqueRegexp: a non-ASCII constraint name lost the unique flag on its
  columns, matching the constraintRegexp fix.

Not covered: a non-ASCII table *name* still fails, because renameTable
locates the name with \b and Go's word boundary is ASCII-only, so it
never matches next to a character like 用. That needs delimiter matching
rather than a wider character class, and is left for its own change.